test(rest): type the unchecked route.handler call sites in rest.test.ts, and measure what the response literals actually are - #15582
Merged
os-litant merged 2 commits intoSep 5, 2026
Conversation
….test.ts
`rest.test.ts` held five `const res = { json: vi.fn(), ... }` literals whose
enclosing `rest` / `route` binding was declared `any`, so `route.handler(req,
res)` checked neither argument. No repair to argument 1 could ever surface
them and `test-typecheck-debt.json` could never see them: they were green
because nothing looked, not because they conformed.
Typing the enclosing binding at all five turns that into a measurement:
- 3 of them (the `filterAppForUser` gate, the dashboard widget gate, and
the `readMeta` helper of the default-config gate) already supplied all
four members `IHttpResponse` requires. They CONFORM — tsc reports nothing
at those three call sites once argument 2 is actually checked.
- 2 of them supplied only `json` and `status`, and go red with exactly the
TS2345 `missing the following properties from type 'IHttpResponse': send,
header` that #13454 repaired and cleared from the ledger. Repaired the
same way: `httpResponseTestDouble()`.
Typing the shared `getPutRoute(rest, path)` helper also exposed 2 further
unchecked calls in the same describe block, at 1930 / 1954, that the census
pattern `const res = { json: vi.fn()` cannot see at all: their response is an
anonymous literal written at the argument position rather than bound to a
`const`. Same class, same two missing members, same repair.
Argument 1 is repaired at all seven with the package's own
`httpRequestForRoute(route!, …)` (#13377), because tsc reports at most one
argument-assignability error per call expression — leaving a request literal
missing `query` / `method` / `path` would simply mask argument 2 again and
put the sites back where they started.
`test-typecheck-debt.json` is untouched and stays empty: the shrink-only
ledger is not the place for an error a fixture can just stop having.
⛔ Not touched, deliberately: the 21 `res as any` sites (#13377 declined that
census for the request half and left 151 sites on purpose), and the mirrored
`makeRes()` fixtures — the three named files plus the in-file `invoke()`
helper at 4141 — where a mirror keeps only the LAST status while `mock.calls`
keeps every one, so substitution would change the assertion rather than the
fixture.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…checked-response-literal-sites
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Sep 4, 2026
os-litant
marked this pull request as ready for review
September 4, 2026 23:33
os-litant
enabled auto-merge
September 4, 2026 23:33
os-litant
deleted the
claude/issue-14356-unchecked-response-literal-sites
branch
September 5, 2026 00:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14356
Test-only. No published surface moves:
packages/rest/tsconfig.jsonexcludes**/*.test.tsfrom the build config, and this diff is one test file.What this PR measures
packages/rest/src/rest.test.tsheld fiveconst res = { json: vi.fn(), ... }literals whose enclosingrest/routebinding was declaredany, soroute.handler(req, res)checked neither argument. No argument-1 error was masking them, noas anysat on the response, andpackages/rest/test-typecheck-debt.json— empty since #13454 — could never see them. They were green because nothing looked, not because they conformed.Typing the enclosing binding is the whole change. The repairs below are what the resulting measurement then required.
Re-derived split, with the card's own controls (measured here, not inherited)
5c9e40aconst res = { json: vi.fn()insrc/rest.test.tsres as anyin the same filetest-typecheck-debt.jsonentries{}{}{}— untouched.handler(acrosssrc/*.test.ts26 = 21 + 5 reproduces exactly. The control moving 284 to 290 across the same window confirms the instrument is live while this population is not.
The five were told apart from the twenty-one by line set, not by eye: the 26 literal line numbers are 1382 … 1762 (21 of them, each with a
res as anyon the following handler call) plus 1900, 1988, 3901, 3975, 4043, which have no cast anywhere. Located by content, since the card's3900 / 3974 / 4042had drifted:const route = rest.getRoutes().find(followed byroute.handler(req, res), with an untyped enclosing binding. The otherrest.getRoutes().find(sites at:696,:1880and:4131are not in this class.The measurement
Typing the enclosing binding at all five and repairing argument 1 with the package's own
httpRequestForRoute(#13377) — becausetscreports at most one argument-assignability error per call expression, so a request literal missingquery/method/pathsimply masks argument 2 again — the test-layer program said, verbatim (the only edit is that the angle brackets ofMock<Procedure>are written as HTML entities, because GitHub's body sanitizer deletes generic-shaped tokens even inside a fence; every other byte is tsc's):Three things in it, in order of how much they change the card:
1. The three sites the card predicted would conform, DO conform. Sites 3901 / 3975 / 4043 — the
filterAppForUsergate, the dashboard-widget gate, and thereadMetahelper of the default-config gate — appear nowhere above. Once argument 2 is genuinely checked, their four-member literals satisfyIHttpResponse. The card's "they would conform if anything looked" was an unmeasured claim; it is now a measurement, and it is true. The expected red did not arrive there.2. The other two of the five are missing
sendandheader. Sites 1900 and 1988 supply onlyjsonandstatus, and go red with exactly the TS2345 signature #13454 repaired and then deleted from this package's ledger. That is the answer to "what are the other two missing".3. A red the card's own instrument could not have predicted — two further unchecked calls at 1930 / 1954. Both sit in the same
describeblock and reach the handler through the samegetPutRoute(rest, path)helper, so typing that helper exposed them. They are not among the 26: their response is an anonymous literal written at the argument position, never bound to aconst, so the census patternconst res = { json: vi.fn()cannot see them at all. Same class, same two missing members. The unchecked-call class in this file is seven, not five; the bound-literal census is a lower bound on it.The repair
Every red above is repaired in the code. ⛔ Nothing was written into
test-typecheck-debt.json: a shrink-only ledger is not where an error a fixture can simply stop having belongs, and an entry here would have re-created one layer up the "clean number that was never earned" this card is about. It stays{}, byte-identical.httpResponseTestDouble()(The 2 remaining ledgered TS2345 in packages/rest are hand-builtIHttpResponseliterals — they were masked by the request literals, not absent #13454), which computes its required-member set from the contract. The two ADR-0112 assertions at 1988 keep assertingcodeandstatus(expect(res.status).toHaveBeenCalledWith(409)plus theMETADATA_CONFLICTcode onres.json).httpRequestForRoute(route!, ...), which derivesmethodfrom the route and materializespathfrom the pattern, so the request cannot disagree with the route it is sent to.1930 / 1954 are a bounded in-place fix, named here rather than left to a reader to find. They are the same defect class as the card, their correct form is pinned by evidence already in the tree (
src/http-response-test-builder.tsplus the two sibling sites #13454 already converted at 2063 / 2088), no other open PR head holds this file, and they add no gate family. They were also not optional: typing the sharedgetPutRoutehelper is what makes 1900 and 1988 checkable at all, and that same edit reds them.Explicitly not done
res as anysites are untouched (count unchanged at 21). A typed IHttpRequest test builder — 4 ledgered TS2345 in packages/rest are hand-built request literals, and the only in-repo alternative isas any#13377 declined that census for the request half and left 151 sites on purpose; this card records that standing decision's shape, it does not reopen it.makeRes()fixture was converted — notanalytics-dataset-dimension-gate.test.ts,meta-public-book-grant.test.tsorrest-batch-size-cap.test.ts. A mirror keeps only the LAST status wheremock.callskeeps every one, so substitution changes what the test asserts rather than the fixture.route.handler(...)call remains unchecked in this file, at line 4141, inside theinvoke(rest: any, ...)helper of theobject API exposureblock. It is a fourth instance of that mirrored-makeResclass, insiderest.test.tsitself rather than in one of the three named files, and it asserts onres.statusCode/res.body. Left alone for the same reason and reported separately.packages/rest/src/rest-server.ts(fix(approvals,rest,types): a stranded decision publishes finalized / decision / runId / repairable beside its 500 #15436) andpackages/types/src/node.ts(fix(types): recognise an aliased package root by the name its declaration promises #15563) were read but never edited.Changeset
None, and none is owed.
pr-automation.yml,Check Changeset,WHICH LEVELroute 2: a PR that releases nothing — "tests-only, and the like" — takes theskip-changesetlabel, which is preferred over an empty changeset. This diff is one*.test.tsfile, excluded from the package's build config and from the published artifact.Verification
All of it on the merged head
c4bde186f53, exit codes captured before any pipe.pnpm --filter '@objectstack/rest^...' build— exit 0 (run first; a staledist/*.d.tslies in both directions).pnpm --filter @objectstack/rest typecheck— exit 0. Its second half prints:check:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json; 0 file(s) / 0 error(s) / 0 pinned signature(s) held in test-typecheck-debt.json.pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2— exit 0, 179 files / 3056 tests passed. Addingmethod/pathto the requests andsend/headerto four responses changed no assertion outcome.pnpm lint— the FULL repo sweep,eslint . --no-inline-config, exit 0. No narrowing claimed.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackon this head, which does not call the tree stale. All green:check:cross-package-test-inputs,check:dispatcher-error-vocabulary,check:doc-authoring,check:logger-receiver-detach,check:objectql-double-limit,check:org-identifier,check:page-declaration-shape,check:published-files,check:slot-lookup,check:test-source-alias,check:type-source-resolution,check:where-matcher,check:nul-bytes,check:driver-memory-census,check-reference-carrier-shape(and self-test),check-ci-filter-parity,check-comment-mask-adoption(and self-test),check-keyed-text-bounds(and self-test),check-platform-object-tenancy-census,check-plugin-teardown-shape,check-registry-log-declared(and self-test),check-system-context-census,check-undeclared-dep-imports.silentfor every card, so they were run rather than read as a clearance:check:authz-resolver,check:error-code-casing,check:filter-alias-parity,check:engine-double-contract,check:error-status-conformance,check:single-claim-paths— all exit 0.🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code